Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow gep instructions to accept bare Python integers as indices. #710

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

trbabb
Copy link
Contributor

@trbabb trbabb commented Mar 21, 2021

New feature: gep instructions can accept Python integers as indices, instead of requiring the client code to tediously construct ir.Constants around each index.

For example:

int32 = ir.IntType(32)
builder.gep(my_object, [int32(5), int32(0), int32(3)])

becomes

builder.gep(my_object, [5, 0, 3])

The width of each converted integer will be chosen to be the smallest number of 8-bit bytes that is a power of two and can represent the value.

It is permissible to intermix Python integers with IR instructions in the index list.

ir.Constants will be automatically constructed around Python integers if they are provided.
The width of each converted integer will be chosen to be the smallest number of 8-bit bytes that can represent the value.
This changes the scheme to use i32s for all GEP indicies converted from Python ints, unless the index is too large to represent with an i32, in which case a wider integer is used.
@esc esc added this to the Version 0.37.0 RC milestone Mar 22, 2021
@esc
Copy link
Member

esc commented Mar 22, 2021

@trbabb thank you for submitting this to the Numba issue tracker. I have added this to the 0.37.0 milestone. Also, it seems like there are some remaining flake8 issues to be resolved.

@trbabb
Copy link
Contributor Author

trbabb commented Mar 22, 2021

Fixed the stray whitespace, but the overlong line is inlined SSA, which that file is already full of, and which I can't really truncate. What do I need to do to prevent that from failing the test?

@esc
Copy link
Member

esc commented Mar 22, 2021

Fixed the stray whitespace, but the overlong line is inlined SSA, which that file is already full of, and which I can't really truncate. What do I need to do to prevent that from failing the test?

This should help: https://flake8.pycqa.org/en/3.1.1/user/ignoring-errors.html#in-line-ignoring-errors

Copy link
Member

@gmarkall gmarkall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for the PR!

I like the idea of this change, but I think the changes to the tests need modifying. Instead of modifying an existing test case, could you add a new test case that tests accepting integers for the gep instruction (and a mix of integers and ir.Constant values) please?

Comment on lines +501 to +502
i_type = types.IntType(bits)
i = i_type(i)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
i_type = types.IntType(bits)
i = i_type(i)
i = types.IntType(bits)(i)

@gmarkall
Copy link
Member

@trbabb Many thanks for the PR and your efforts so far!

I'm spending some time going through the llvmlite PR backlog, and I have a couple of questions on this PR:

  • Do you plan to continue with this PR? If it's difficult for you to work on this PR for any reason, please let me know and we can look into whether / how to proceed further.
  • If you're keen to continue - do you need any further advice on how to proceed?

@sklam sklam modified the milestones: 0.39.0 RC, PR Backlog Jun 1, 2022
@esc esc removed this from the PR Backlog milestone Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants